Skip to content

Conversation

@0xced
Copy link
Contributor

@0xced 0xced commented Jun 13, 2019

This is useful to store exception details when abandoning a message and still keeping the convenience of registering a message or session handler with AutoComplete = true

Ported from Azure/azure-service-bus-dotnet#671 which is now archived.

This is useful to store exception details when abandoning a message and still keeping the convenience of registering a message or session handler with AutoComplete = true
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@0xced
Copy link
Contributor Author

0xced commented Jun 26, 2019

@nemakam Could you please follow up on this when you get some time please?

@nemakam
Copy link
Contributor

nemakam commented Jul 12, 2019

@0xced , again, thanks for raising PR, but I don't see the necessity for this change. I stand by my comments that I mentioned in the previous PR.
The only advantage of this complicated API is that in the user callback you need not call CompleteAsync\AbandonAsync yourself, and you want the pump to take care of it. That defeats the whole purpose of having the option of AutoComplete in HandlerOptions.
I'd like to understand the hesitation of why disabling autoComplete and taking care of that within your handler is a difficult task?

One of the main reason why I am very hesitant to approve this PR is that the dictionary that is received from user is only used on user callback exceptions. That API doesn't look clean. And also, I don't know how many people would this even benefit. I am of the opinion more people would be confused by this than people who need this feature.

I am inclined to close this PR. If we can come to an alternate conclusion later, I will open it again.

@nemakam nemakam closed this Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants